Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[8.x] username parameter in from method should be nullable #1818

Closed
wants to merge 3 commits into from
Closed

[8.x] username parameter in from method should be nullable #1818

wants to merge 3 commits into from

Conversation

mho22
Copy link

@mho22 mho22 commented Jul 11, 2024

Based on issue #1815, username parameter in from method should be nullable in DiscordMessage.php on line 13 :

src/Notifications/Channels/Discord/DiscordMessage.php

protected string $username = 'Laravel Backup';
 
...
public function from(string $username, string $avatarUrl = null): self
{
        $this->username = $username;
...

@mho22 mho22 changed the title [8.x] $username parameter in from method should be nullable [8.x] username parameter in from method should be nullable Jul 11, 2024
@mho22 mho22 closed this Jul 11, 2024
@mho22 mho22 reopened this Jul 11, 2024
@RVxLab RVxLab mentioned this pull request Jul 14, 2024
Closed
@Nielsvanpach
Copy link
Member

I think this is fixed in this PR: #1821

Feel free to reopen if not the case!

@mho22
Copy link
Author

mho22 commented Jul 22, 2024 •
edited
Loading

@Nielsvanpach, the PR is applicable to version 9, not version 8. Therefore, I will need to update to version 9, but this will cause issue #1813 to occur.

@RVxLab
Copy link
Contributor

RVxLab commented Aug 7, 2024

Much like I mentioned in my comment in #1817, that won't fix the issue that was fixed in #1821

@Nielsvanpach would you (or Freek) accept a PR to backport that fix to v8? I don't know what your typical stance is on older major versions.

If not, then @mho22 you would be better off forking and maintaining it yourself.

@freekmurze
Copy link
Member

accept a PR to backport that fix to v8?

Yup, send a PR to the v8 branch and I'll tag a new release.

@RVxLab RVxLab mentioned this pull request Aug 7, 2024
Merged
@RVxLab
Copy link
Contributor

RVxLab commented Aug 7, 2024

PR has been created.

@RVxLab
Copy link
Contributor

RVxLab commented Aug 7, 2024

@mho22 Fix for the Discord notifications has been backported. If you update to ^8.8.2 it'll start working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nielsvanpach Nielsvanpach Awaiting requested review from Nielsvanpach

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mho22 @Nielsvanpach @RVxLab @freekmurze